Skip to content

fix(distributed): count staging verification as progress, not as a stall#11026

Merged
mudler merged 1 commit into
masterfrom
fix/staging-verify-progress
Jul 21, 2026
Merged

fix(distributed): count staging verification as progress, not as a stall#11026
mudler merged 1 commit into
masterfrom
fix/staging-verify-progress

Conversation

@localai-bot

Copy link
Copy Markdown
Collaborator

Follow-up to #11019 (merged as b700a78ae). Testing that change on the live cluster surfaced a false positive in the stall window.

The gap

The stall window observes upload bytes via withStagingCallback. But the staging path has a phase that does real work while moving zero upload bytes: the resumable-upload verify phase. When a shard is already present on the worker from an earlier attempt, the frontend HEADs it, hashes the local copy to confirm it matches, and skips the transfer.

Observed staging a 70 GB model with 56 GB already present from a previous attempt:

17:27:34 INFO Upload skipped (file already exists with matching hash) ...
17:28:20 INFO Upload skipped (file already exists with matching hash) ...
17:29:07 INFO Upload skipped (file already exists with matching hash) ...
17:29:53 INFO Upload skipped (file already exists with matching hash) ...
17:30:41 INFO Upload skipped (file already exists with matching hash) ...
17:31:27 INFO Upload skipped (file already exists with matching hash) ...

~45s per skipped ~4 GB shard, six-plus consecutive minutes with no bytes uploaded at all. That behaviour is correct and desirable — it is what makes resume work — but from the deadline's point of view it was indistinguishable from a stall.

At 45s per shard this sits comfortably inside the 5m window, so the run in flight was fine. The problem is the scenario #11019 exists for: at 600 GB, a single shard can plausibly take longer than 5 minutes to hash. The guard would then fire during verification of a transfer that is working perfectly — a false positive landing precisely in the case the work exists to enable.

Mechanism verified

probeExisting() HEADs the worker, then calls downloader.CalculateSHA(). The staging progress callback is only consulted at StagingProgressFromContext(ctx) inside doUpload(), which the skip path never reaches. So observeLoadProgress was called zero times for the entire verify phase. The gap is real.

A second, worse bug in the same path

Writing the test exposed something beyond the reported issue: CalculateSHA consults no context at all. An expired cold load kept hashing to completion, compared the hashes, and returned success — reporting a file as staged on a dead load. The failure surfaced only on the next file, whose HEAD died immediately.

This is exactly what the red test shows: it fails on shard 3, not shard 1, which also explains the production error shape (a run of successful skips, then an abrupt context deadline exceeded on a later file).

Fix

Hash in 1 MiB chunks via hashFileWithActivity(), which:

  1. ticks the cold-load deadline once per chunk, so hashing counts as progress; and
  2. checks ctx.Err() per chunk, so an expired load aborts instead of silently succeeding.

A successful HEAD also ticks the deadline: a 200 carrying a content hash is proof the worker is serving right now.

Why this does not make a dead transfer look alive

Considered and rejected: simply widening the window (reintroduces the size cliff #11019 removes) and a coarser per-file "staging is doing something" signal (a single 600 GB shard would still look stalled for hours — too coarse to distinguish the two cases, which is the whole point).

Hash progress is a safe signal because:

  • it is bounded, terminating work proportional to file size — it cannot extend the hold indefinitely;
  • in probeExisting it runs only after a successful HEAD proved the worker was serving;
  • the pre-upload hash runs once per EnsureRemote, outside the retry loop, so it can mask at most one file-hash duration; and
  • the 24h absolute cap still bounds the whole hold regardless.

A genuinely wedged worker produces no HEAD response and no hashing, so it is still caught by the stall window — covered by a spec.

Tests

TDD, Ginkgo/Gomega, in core/services/nodes/file_stager_verify_deadline_test.go, exercising the real HTTPFileStager against an httptest worker. Red first, on behaviour:

[FAILED] verifying shard 3 does real work and must not be mistaken for a stall
Unexpected error:
    uploading /tmp/.../shard-3.safetensors to node nvidia-thor:
    Put "http://127.0.0.1:46045/v1/files/shard-3": cold-load deadline expired

Ran 3 of 406 Specs
FAIL! -- 2 Passed | 1 Failed

Specs: a run of verified-and-skipped shards uploading zero bytes must survive; the verify path must not silently succeed after the deadline expired; a silent worker must still be killed promptly.

Note on test methodology: my first attempt at this test was a false green — the window was larger than the hash time, so it passed without any fix. The committed version was recalibrated until it failed for the right reason, and the shard sizes were then reduced to stay friendly to /tmp.

Verification

Command Exit
go build ./core/... ./pkg/... 0
go test ./core/services/nodes/ (full suite, 406 specs) 0
make lint 0 (0 issues.)

Not verified: the 600 GB slow-hash scenario was not reproduced — the specs simulate it with scaled-down budgets (20ms window vs 5m) against real hashing of small shards. The production log above is from the cluster, not from this branch.

Docs updated in docs/content/features/distributed-mode.md per the docs-with-code rule.

Testing the progress-based cold-load deadline on the live cluster surfaced a
false positive. The stall window observed UPLOAD bytes only, but the staging
path has a phase that does real work while moving zero upload bytes: the
resumable-upload verify phase.

When a shard is already present on the worker from an earlier attempt, the
frontend HEADs it, hashes the local copy to confirm it matches, and skips the
transfer. Staging a 70 GB model with 56 GB already staged:

  17:27:34 INFO Upload skipped (file already exists with matching hash) ...
  17:28:20 INFO Upload skipped (file already exists with matching hash) ...
  17:29:07 INFO Upload skipped (file already exists with matching hash) ...
  ... six-plus consecutive minutes, no bytes uploaded at all

~45s per skipped ~4 GB shard. That is correct and desirable - it is what makes
resume work - but it was indistinguishable from a stall. At 45s per shard it
sits inside the 5m window, so the run in flight was fine; the problem is the
600 GB scale this machinery exists to enable, where one shard can plausibly hash
for longer than the window. The guard would then fire during verification of a
transfer that is working perfectly.

Verified mechanism: probeExisting() HEADs the worker and then calls
downloader.CalculateSHA(). The staging progress callback is only consulted
inside doUpload(), which the skip path never reaches, so observeLoadProgress was
called zero times for the whole verify phase.

Verification exposed a second, worse bug in the same path: CalculateSHA consults
no context at all. An expired cold load kept hashing to completion, compared the
hashes, and returned success - reporting a file as staged on a dead load. The
failure only surfaced on the NEXT file, whose HEAD died immediately. That is
exactly the shape of the red test here, which fails on shard 3.

Fix: hash in 1 MiB chunks via hashFileWithActivity(), ticking the cold-load
deadline per chunk and checking ctx per chunk. A successful HEAD also counts,
since a 200 with a content hash proves the worker is serving right now.

Counting hash progress does not make a dead transfer look alive: hashing is
bounded, terminating work proportional to file size, in probeExisting it runs
only after a HEAD proved the worker was up, and the 24h absolute cap still
bounds the whole hold. The alternative of simply widening the window was
rejected - it would reintroduce the size cliff this work removes.

Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
Assisted-by: Claude Code:claude-opus-4-8[1m] [Read] [Edit] [Bash]
@mudler
mudler merged commit a4a181d into master Jul 21, 2026
21 of 22 checks passed
@mudler
mudler deleted the fix/staging-verify-progress branch July 21, 2026 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants